Depend on tar-0.6.3.0 by mpickering · Pull Request #10120 · haskell/cabal

mpickering · 2024-06-17T15:46:44Z

tar-0.6.3.0 has much improved performance of deserialising .tar index
which has significant ramifications for the start-up time of
cabal-install.

See #10110

ffaf1 · 2024-06-17T16:38:19Z

I wonder if this compat module can be axed then.

cabal/cabal-install/src/Distribution/Client/Compat/Tar.hs

Lines 4 to 18 in e1f73a4

    
           {- FOURMOLU_DISABLE -} 
        
           module Distribution.Client.Compat.Tar 
        
             ( extractTarGzFile 
        
           #if MIN_VERSION_tar(0,6,0) 
        
             , Tar.Entry 
        
             , Tar.Entries 
        
             , Tar.GenEntries (..) 
        
             , Tar.GenEntryContent (..) 
        
             , Tar.entryContent 
        
           #else 
        
             , Tar.Entries (..) 
        
             , Tar.Entry (..) 
        
             , Tar.EntryContent (..) 
        
           #endif 
        
             ) where

I also wonder: are we introducing some indirect constraint on base?

ulysses4ever · 2024-06-17T17:10:33Z

It's great to hear about tar advancements. Reacting to these with bumping the lower bound seems too harsh to me.

mpickering · 2024-06-17T17:14:42Z

@ffaf1 A good question, tar builds with base >= 4.11 which goes back to 8.4.4 at least.

@ulysses4ever It's up to the maintainers what they wish to do, but older versions of tar have a non-performant implementation of deserialise so if you build cabal-install against them then you will suffer slower start-up times.

I will at least bump the index state for release and bootstrap jobs (which will then pick tar-0.6.3)

ulysses4ever · 2024-06-17T17:24:50Z

older versions of tar have a non-performant implementation of deserialise so if you build cabal-install against them then you will suffer slower start-up times.

I understand. I don't see why you want to disallow that. If your plan allows a newer version of tar, the solver will pick it up anyway. But otherwise, with this change, you just get stuck.

Updating the index state makes sense, thank you.

mpickering · 2024-06-17T17:28:07Z

I suppose I see it in the same way as if a previous version of the package had a serious bug, you wouldn't want to allow your users to build against that package. I'll remove the tight lower bound once CI passes.

tar-0.6.3.0 has much improved performance of deserialising .tar index which has significant ramifications for the start-up time of cabal-install. See haskell#10110

Using the updated version of this library improves performance of cabal-install so it's worthwhile to make sure the bootstrap plans use this version.

mpickering · 2024-06-18T08:22:34Z

I have removed the more aggressive lower bound on cabal-install now (which doesn't seem like the best thing to do in my opinion).

So can someone please approve the remaining changes which bumps the index state and regenerates the bootstrap plans to include tar-0.6.3.0.

Mikolaj

LGTM

Mikolaj · 2024-06-18T08:52:14Z

If nobody objects, let's expedite this PR and the backport.

ffaf1 · 2024-06-18T10:28:29Z

What about the Compat module @mpickering? Is it still needed?

Mikolaj · 2024-06-18T11:53:38Z

@ffaf1: we haven't bumped tar beyond (0,6,0), after all, so it seems it is needed? I guess we could have made such a modest bump, but we are in a hurry this time.

I haven't heard objections, so I'm force-merging.

Mikolaj · 2024-06-18T11:55:50Z

@mergify backport 3.12

mergify · 2024-06-18T11:56:01Z

backport 3.12

✅ Backports have been created

Details

#10123 Depend on tar-0.6.3.0 (backport #10120) has been created for branch 3.12 but encountered conflicts

Mikolaj · 2024-06-18T11:57:43Z

Merged. @mpickering: thank you again.

And we have conflicts in the backport. :(

* Bump index state to allow tar-0.6.3.0 tar-0.6.3.0 has much improved performance of deserialising .tar index which has significant ramifications for the start-up time of cabal-install. See #10110 (cherry picked from commit 827256a) * Update bootstrap plans to include tar-0.6.3 Using the updated version of this library improves performance of cabal-install so it's worthwhile to make sure the bootstrap plans use this version. (cherry picked from commit 4d787e1) # Conflicts: # bootstrap/linux-8.10.7.json # bootstrap/linux-9.0.2.json # bootstrap/linux-9.2.8.json # bootstrap/linux-9.4.8.json # bootstrap/linux-9.6.4.json # bootstrap/linux-9.8.2.json * fixup! Update bootstrap plans to include tar-0.6.3 --------- Co-authored-by: Matthew Pickering <matthewtpickering@gmail.com> Co-authored-by: brandon s allbery kf8nh <allbery.b@gmail.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

mpickering force-pushed the wip/tar-bound branch from 24cb3cb to 111291e Compare June 17, 2024 15:50

geekosaur approved these changes Jun 17, 2024

View reviewed changes

ulysses4ever mentioned this pull request Jun 17, 2024

3.12.1.0 — a minor release of Cabal and friends #9885

Closed

18 tasks

mpickering added 2 commits June 18, 2024 09:20

Bump index state to allow tar-0.6.3.0

827256a

tar-0.6.3.0 has much improved performance of deserialising .tar index which has significant ramifications for the start-up time of cabal-install. See haskell#10110

Update bootstrap plans to include tar-0.6.3

4d787e1

Using the updated version of this library improves performance of cabal-install so it's worthwhile to make sure the bootstrap plans use this version.

mpickering force-pushed the wip/tar-bound branch from c56149a to 4d787e1 Compare June 18, 2024 08:21

Mikolaj approved these changes Jun 18, 2024

View reviewed changes

Mikolaj added merge me Tell Mergify Bot to merge attention: needs-backport 3.12 labels Jun 18, 2024

Mikolaj added the merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days label Jun 18, 2024

mergify bot merged commit 1c00507 into haskell:master Jun 18, 2024

mergify bot mentioned this pull request Jun 18, 2024

Depend on tar-0.6.3.0 (backport #10120) #10123

Merged

geekosaur removed the attention: needs-backport 3.12 label Sep 13, 2024

Conversation

mpickering commented Jun 17, 2024

Uh oh!

ffaf1 commented Jun 17, 2024

Uh oh!

ulysses4ever commented Jun 17, 2024

Uh oh!

mpickering commented Jun 17, 2024

Uh oh!

ulysses4ever commented Jun 17, 2024

Uh oh!

mpickering commented Jun 17, 2024

Uh oh!

mpickering commented Jun 18, 2024

Uh oh!

Mikolaj left a comment

Choose a reason for hiding this comment

Uh oh!

Mikolaj commented Jun 18, 2024

Uh oh!

ffaf1 commented Jun 18, 2024

Uh oh!

Mikolaj commented Jun 18, 2024

Uh oh!

Mikolaj commented Jun 18, 2024

Uh oh!

mergify bot commented Jun 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Backports have been created

Uh oh!

Mikolaj commented Jun 18, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

mergify bot commented Jun 18, 2024 •

edited

Loading